Add RegisterTools API to McpClient for pre-populating tool cache#1590
Conversation
This enables MCP clients to send Mcp-Param-* HTTP headers without requiring a prior ListToolsAsync call, addressing issue modelcontextprotocol#1577. Changes: - Add RegisterTools abstract method to McpClient with XML documentation - Implement RegisterTools in McpClientImpl with thread-safe ConcurrentDictionary for registered tool name tracking - Modify ToolCacheClearing to preserve registered tools across ListToolsAsync calls while clearing server-discovered tools - Add fast path optimization when no tools are registered - Validate x-mcp-header annotations on registered tools Tests: - 11 unit tests covering cache interaction scenarios - 3 HTTP integration tests verifying Mcp-Param-* headers are sent without ListToolsAsync
Adding an abstract member to McpClient is a breaking change (CP0005) because existing subclasses would fail to compile. Changed to virtual with a default no-op implementation that validates the argument.
halter73
left a comment
There was a problem hiding this comment.
Thanks for tackling this! CI checked, and we're ahead of the other tier 1 SDKs here. TypeScript (modelcontextprotocol/typescript-sdk#2069) only landed Mcp-Method/Mcp-Name, Python has no SEP-2243 implementation at all, and Go shipped x-mcp-header (modelcontextprotocol/go-sdk#915) with the same silent-omission gap that #1577 calls out. So this PR is paving the path for everyone.
One thing I'd like to see addressed before merge but couldn't comment on inline because it's out of the diff range: the silent-miss path in McpClientImpl.SendRequestAsync. If the caller forgets to call either RegisterTools or ListToolsAsync, the cache lookup fails, no Tool is attached, and StreamableHttpClientSessionTransport ships the tools/call with no Mcp-Param-* headers — no warning, no log, no error. The server then sees a tools/call with no infra headers and may misroute (e.g. a proxy keyed on Mcp-Param-Region). Worth a LogDebug (or LogInformation, gated on the negotiated protocol version supporting SEP-2243) on the cache miss so it's at least diagnosable. Other SDKs have the same gap, but logging is cheap and a real footgun-mitigator.
Follow-up, not for this PR: I'd also like us to eventually offer a fully stateless way to flow parameter values into Mcp-Param-* headers — e.g. a Tool (or just a param → header map) passed directly on CallToolAsync / CallToolRequestOptions, with no client-wide cache mutation. The cache-priming approach in this PR is the right answer for the "I already know the schema from a previous session" scenario, but for callers who want per-call control (multi-tenant proxies, request-scoped routing, callers who don't want any client-global mutable state) the stateless variant is a cleaner fit and dovetails with what the Python folks are circling in modelcontextprotocol/python-sdk#1509. I'm happy to file a separate issue once this lands.
Clarifies that this is client-side cache priming, not server-side tool registration. Renamed method, test classes, and test files.
…hen-commit - Add RemoveKnownTools(IEnumerable<string>) to remove specific known tools - Add ClearKnownTools() to remove all known tools - Change AddKnownTools to validate all tools first, then commit all-or-nothing - Throw ArgumentException on invalid x-mcp-header annotations instead of silently skipping (ToolRejected still fires for logging parity) - Remove trailing blank line in McpClient.cs - Add 7 unit tests for Remove/Clear scenarios - Add HTTP integration test for RemoveKnownTools header verification
Validate all tool names for null before removing any, matching the all-or-nothing pattern used in AddKnownTools.
Add 'Pre-loading tool definitions on the client' section to docs/concepts/tools/tools.md covering usage, cache behavior, removal APIs, and validation semantics.
Prevents subclasses that don't override from silently swallowing calls. All three methods (AddKnownTools, RemoveKnownTools, ClearKnownTools) now throw with a descriptive message.
Clarify that known status is sticky for the McpClient lifetime and point to RemoveKnownTools/ClearKnownTools for explicit removal.
- Staleness: register → server returns [] → headers still sent - Partial-failure atomicity: [valid, invalid, valid] → nothing cached - Null element atomicity: null at index 1 → nothing cached - Last-write-wins: re-register with schema B → headers reflect B
When SendRequestAsync handles a tools/call request and the tool is not found in the cache, log a Debug message suggesting AddKnownTools or ListToolsAsync to populate the cache. This makes missing Mcp-Param-* headers diagnosable without breaking existing behavior.
|
@halter73 thanks for your feedback. I believe I addressed all your feedback. The remaining open discussions looks can be addressed outside this PR. Let me know if you have any more feedback. |
Log a Warning (instead of Debug) when a tools/call request has no cached tool definition, but only for StreamableHttpClientSessionTransport where Mcp-Param-* headers are relevant. Pipe/stdio transports do not emit this warning since headers do not apply. Added tests: - Pipe transport: cache miss does NOT log a warning - HTTP transport: cache miss DOES log a warning
Summary
This PR adds
AddKnownTools,RemoveKnownTools, andClearKnownToolsAPIs toMcpClientthat allow pre-populating the internal tool cache with tool definitions. This enables MCP clients to sendMcp-Param-*HTTP headers (based onx-mcp-headerschema annotations) without requiring a priorListToolsAsynccall.Fixes #1577
Changes
API Additions on
McpClientAddKnownTools(IEnumerable<Tool>)— registers tool definitions in the client's tool cache with all-or-nothing validationRemoveKnownTools(IEnumerable<string>)— removes specific known tools by nameClearKnownTools()— removes all known toolsBase virtual methods throw
NotSupportedException(not abstract, to preserve API compatibility withPackageValidationBaselineVersion=1.0.0).Implementation Details
_toolCacheused byListToolsAsyncConcurrentDictionary<string, byte>tracks known tool names (used as a concurrent set)ToolCacheClearing(invoked byListToolsAsync) only removes server-discovered tools; known tools survive cache clears_toolCache.Clear()is called directlyx-mcp-headercorrectness before any are added;ArgumentExceptionthrown on invalid schemasLogDebugon cache miss duringtools/callto help diagnose missingMcp-Param-*headersCache Interaction Behavior
_registeredToolNamesand_toolCacheDocumentation
docs/concepts/tools/tools.mdTests
Unit Tests (20 tests in
McpClientAddKnownToolsTests.cs)HTTP Integration Tests (6 tests in
AddKnownToolsHeaderTests.cs)AddKnownTools->CallToolAsync(NOListToolsAsync) -> verifyMcp-Param-RegionandMcp-Param-Priorityheaders received by serverListToolsAsynccache clearRemoveKnownTools-> no headers sent after removal